-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
POC: Handler for preventing the load of incompatible "for Core" plugins #5855
base: trunk
Are you sure you want to change the base?
POC: Handler for preventing the load of incompatible "for Core" plugins #5855
Conversation
af6d001
to
c8e80a0
Compare
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
fcebb28
to
eb79432
Compare
Changes the static class design to function variable design. Why? To eliminate BC concerns by encapsulating the code for usage only during the plugin loading loops. Once these loops are done, the variables are unset. This removes the code from global space, i.e. meaning the code can't be accessed or used.
0f5d729
to
644e2a1
Compare
Removes changes outside of the wp-settings.php file. Simplifies the code (less is better). Collects the incompatible plugins from the loader loops. After all plugins are loaded, then handles updating the options, i.e. does it once instead of doing it for each plugin loader loop.
644e2a1
to
d2ec763
Compare
$plugins_dir_strlen = strlen( WP_PLUGIN_DIR . '/' ); | ||
} | ||
|
||
$plugin = substr( $plugin_absolute_path, $plugins_dir_strlen ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why substr()
rather than using str_replace()
?
It's more performant. See https://3v4l.org/TbQ9U.
Introduces _get_plugin_wp_min_compatible_version(). By providing a global function, for-core plugins, such as Gutenberg, can use it to check the minimum compatible version. It provides a more foolproof way for the plugin to handle its response.
Doing a write to the database will have a performance hit. To avoid the hit, removes the new "wp_force_deactivation_incompatible_plugins" option in favor of hooking a callback into `'admin_notice'` and passing the global into the callback via `use()`. Isn't the global unset() within wp-settings.php? Yes. It's done to avoid leaking the handler out of the file (for future BC). Doesn't unsetting the global cause it to be unavailable to the hooked lambda? No. Notice, the global is passed to the lambda (i.e. anonymous function) via `use()`. What does this do? In PHP, the _value_ of the passed variable gets inherited within the anonymous function is it (the anonymous function) is defined, not when it (the anonymous function) is called. See the PHP Manual https://www.php.net/manual/en/functions.anonymous.php#functions.anonymous#example-541 >// Inherited variable's value is from when the function >// is defined, not when called See it in action https://3v4l.org/1BocJ.
d2ec763
to
1dc3bb9
Compare
Switches from not directly update the active plugins options to using deactivate_plugins() via "admin_init". Reverts the performance commit of using a lambda for the admin notice as the notices need to persist to ensure they are rendered in wp-admin. The option path persists between page loads, so that when a user logs in or moves from frontend to backend, the notice is shown.
This PR is a Proof of Concept (POC). DO NOT MERGE IT YET.
What problem does it solve?
Plugins that are meant to be merged into Core (such as Gutenberg) are (or will be) part of Core. As the plugins are developed and released, older versions may not be compatible with specific WordPress versions. These "incompatible versions" can cause fatal errors and/or unexpected behaviors for users.
Currently Core detects and deactivates incompatible Gutenberg plugin versions. But not all updates are done using Core's upgrader. For those sites, users could have issues.
Currently Core allows an incompatible version of Gutenberg to be (re)activated. This act could cause issues which in the worse case is a fatal error that locks the user out of their site requiring manual intervention.
Gutenberg is a great example for this problem statement. Released versions include experiments and iterations of features that some day may come to Core. As those features evolve within the plugin, incompatibilities and backward compatibility issues will happen. Once a feature is merged into Core, that feature may conflict with older versions of the plugin. Worse case scenario is a fatal error.
A solution is needed within Core to detect incompatible Core plugin versions and then deactivate and not load each.
What does this PR do?
It inserts compatibility logic within the early early loading of activated plugins. Each active incompatible "for Core" plugin found is flagged and not loaded.
"for Core" plugins are hardcoded, i.e. pre-identified and known.
Trac ticket:
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.